Conversation
Browser Dialer fix and new HTTP methods and cookies support. OPTIONS and Access-Control-Allow-* for it. Configurable MaxHeaderBytes limit on the server and strict scMaxEachPostBytes chunking. Get rid of *-Upstream and *-Length headers and *_upstream cookie which may be used as a signature for the XHTTP detection. UplinkDataPlacement auto to support different placements through different CDN's in the single inbound on the server side. Fix XPadding in cookies on the server side.
|
@paqx 看一下 |
|
I can also test this tomorrow to see if it fixes the xhttp bugs. |
|
I reviewed the changes. They look good at a first glance, but there are a lot of them, so I need some more time for thorough testing. |
|
Would you please research more into this line? Firefox complains on CORS, but only for OPTIONS method. The specification technically mentions that * exists as a syntax option on some CORS headers, but in practice browsers do not reliably treat it as a wildcard for allowed methods — and official examples and docs always expect a concrete list of methods." Possible bug? Better to set explicitly to GET, POST, OPTIONS? |
…rol-Request-* in XHTTP.
With Firefox 148.0 I can only reproduce this when some of the {padding,data}Placement is cookie and browser dialer is opened on the localhost. But to use cookies you must open dialer on the same origin with the xray server and then there is no OPTIONS. MDN says that * (wildcard) valid for requests without cookies https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Access-Control-Allow-Methods I changed the value of the Access-Control-Allow-Methods to the value of the Access-Control-Request-Method for OPTIONS and to the current method for others. Try it out. |
I test on XHTTP VLESS TLS + Windows 10 + v2rayN client (TUN mode), Firefox 148.0. I type http://127.0.0.1:8080, not localhost:8080 (don't know if it's important here). So... I tried adding explicit allow methods (GET, POST, OPTIONS) myself before I saw your new code. It did help with cors complaints. But... I started to get lots of 400 responses. And it seems the reason is this in firefox console: I have the dialer working good in chrome and in edge even with your initial PR. But they set referrer policy to unsafe-url. In firefox it switches to strict-origin-when-cross-origin. I tried to disable ETP and other security in firefox, but didn't help. Probably not worth it for now for just firefox? If you can come up with some idea how to make it work without cross-origin path in referrer, I am ready to test. Otherwise I suggest you restore your previous version of this PR and just forget about it. I assume if firefox didn't switch to strict-origin, there would be no problem with allowed methods * |
|
@Kapkap5454 I reproduced your problem. It seems that you use the default settings with the XPadding in the Referer. Here Firefox developers says that since version 93 "Firefox will always trim the HTTP referrer for cross-site requests, regardless of the website’s settings". I think we can do nothing with this.
I have two ideas how to not make cross-origin requests at all.
|
Disable favicon.ico.
|
|
|
No one pointed out any issues in my code yet. What @Kapkap5454 wrote about was a Firefox problem, not mine, and can be resolved with new obfuscation settings. |
|
I haven't been able to test it yet. It's just too difficult to fit a test set-up into my existing infastructure. @26X23 I have a question about this part: Isn't it going to prevent sending uplink requests with body using methods other than "POST", "PUT", "PATCH"? |
@paqx Yes, because as far as I know only this methods supports body. Go http client will not send body with GET method even if you pass body into Request. So with uplinkHTTPMethod=GET and uplinkDataPlacement=body nothing works, but without this check the user doesn't know why. |
|
Got it. But there're other methods like DELETE. Normally, they are not used with a body, but I tested DELETE for uplink and it seemed to work. Also, I saw some non-standard methods. For example, CDNVideo has MOVE, COPY, LOCK, UNLOCK, etc. I thought that other CDNs might have their own non-standard methods that could accept the body and let users bypass restrictions if the CDN blocks POST, PUT, PATCH. That's why I didn't want to hard code specific uplink methods. Maybe it would better to let users specify any uplink method? What do you think? |
|
@paqx Thanks. I removed this check.
I was wrong with this. The go http client can send the body with any method. The problem was with the browser dialer and GET/HEAD with body. But the user still can see the error in the browser console. |
|
@26X23 I tested latest version on both client and server (well, before the changes you did today). It works correct with edge. With firefox no cors error now, just same 400 responses from server (as we expected). I do use default settings for xhttp. Mainly because I don't use cdn. And I dont completely understand those settings. So I try to trust the defaults. Works for now. I did try xPaddingObfsMode": true and "xPaddingPlacement": "header" in client. It doesn't help. I know your wrote I should also change on the server. But I have a few other users and I am not sure adding these settings will or wont break connection for them. The main question I have, if all this can be fixed for firefox by moving padding to header. Maybe it's wise to make it the default setting? So firefox also would work out of the box. Or it has drawbacks and there is reason padding is placed in referrer? |
It will break configuration of current users if they will not update the server and client simultaneously.
RPRX wrote that "放 Referer 是为了防止 Browser Dialer 产生不必要的 OPTIONS 请求" (autotranslated to "The Referer field is used to prevent the Browser Dialer from generating unnecessary OPTIONS requests"). He means that when we add non-standard header into the request browser will send extra OPTIONS request with Access-Control-Request-Headers: YouCustomHeader before cross-origin POST request, and since Referer is the standard header, there is no need to send OPTIONS for it. But I don't know any other standard header that can be freely changed on a per-request basis. |
We may expect sooner or later other browsers might switch to stricter mode in similar situations and browser dialer will just break everywhere.... At least for anyone using default settings. I’ll leave it to the developers’ discretion. |
|
I agree that browsers will probably switch to a stricter mode and the current browser dialer implementation will eventually stop working. If the padding is placed in a custom header by default, the browser will make a preflight request, but the server's response could contain the Access-Control-Max-Age header indicating for how long the preflight results can be cached. Browsers cap the max-age (varies by browser/version). Google Chrome seems to be the worst (max 600 seconds). But it's still 10 minutes and in my opinion one preflight request every 10 minutes should not do much harm. |
This preflight results are cached by path. By default paths are different in every request due to the session id and sequence number. To utilize cache all variadic parts should also be moved from the path to the headers. |
Browser Dialer was broken in XHTTP. I fixed it and added support for the new HTTP methods and cookies.
Added OPTIONS support and some Access-Control-Allow-* headers for browser dialer on the server side.
To reduce the number of OPTIONS requests one can access browser dialer from the XHTTP domain (e.g. cdn.example.com). To hide content of the dialer from the CDN one can create simple loader script, e.g. https://cdn.example.com/loader.html, which will load dialer from the specified url and replaces itself by dialer's content.
Nginx by default has very small limits for header buffers (client_header_buffer_size 1k and large_client_header_buffers 4 8k). And "A request header field cannot exceed the size of one buffer". So it is crucial to strictly follow this limits with the help of scMaxEachPostBytes and uplinkChunkSize settings when using packet-up via GET with payload in headers. But pipe.WriteMultiBuffer in Dial can write any amount of data when pipe isn't full, so after merging ReadMultiBuffer can return more data than allowed by scMaxEachPostBytes. So this data must be split into chunks after reading from pipe in Dial.
Also reduce the default value for the UplinkDataKey from 4 KiB to 4 KB, because 2x(header name + colon and space + 4 KiB + CRLF) will not fit into the default 8 KiB buffer.
Also select uplinkChunkSize randomly from some range to reduce detection.
Allow scMaxEachPostBytes less than 8 KiB and allow to increase MaxHeaderBytes from 8 KiB. Their defaults where contradictive.
Get rid of *-Upstream and *-Length headers and *_upstream cookie which may be used as a signature for the XHTTP detection. Length is not needed, it can be determined from data. Upstream is not needed because it could be determined from the sequence number presence in the packet-up via GET and is true in all other methods like POST, PUT, PATCH, etc.
Added UplinkDataPlacement=auto to support different placements through different CDN's in the single inbound on the server side. (Similar to Mode=auto)
Fix XPadding in cookies on the server side. It was not added at all when xPaddingPlacement=cookie.
Allow the only one of the sessionPlacement or the seqPlacement to be path.
P.S. It is better to view diff without whitespace changes.